Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes for Java module graph (GH4227) #5668

Merged
merged 2 commits into from
Mar 17, 2023
Merged

Fixes for Java module graph (GH4227) #5668

merged 2 commits into from
Mar 17, 2023

Conversation

neilcsmith-net
Copy link
Member

Fixes #4227 - Java module graph component failing to display with a NPE. This is caused by the same issue as fixed in #4780 whereby the module tree is no longer available from cc.getCompilationUnit().getTypeDecls() and should be obtained by cc.getCompilationUnit().getModule(). Need to look across the code base for other instances of this problem.

Second commit fixes a stack overflow noticed in the search function on the graph. This seems to be due to circular dependencies being reported in the underlying dependency graph - probably some other issues to look at there, but at least it doesn't blow up for now!

@neilcsmith-net
Fix DependencyCalculator in module info graph.
e8be08d 
Need to use getModule() rather than getTypeDecls() to access ModuleTree in dependency calculation.

Fixes GH4227.
@neilcsmith-net
Filter out nodes already in path inside SearchVisitor.
ba654d0 
Fix a stack overflow in the module graph search due to nodes being
visited multiple times.
@neilcsmith-net neilcsmith-net added the Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) label Mar 16, 2023
@neilcsmith-net neilcsmith-net added this to the NB18 milestone Mar 16, 2023
@neilcsmith-net neilcsmith-net requested review from mbien and jlahoda March 16, 2023 10:58
@mbien
Copy link
Member

mbien commented Mar 16, 2023

TIL that NB has a module graph for the java module system

@mbien
Copy link
Member

mbien commented Mar 17, 2023

while clicking around I found a NPE, however this isn't caused by this PR (click on the "Show Readability" icon on the right)

java.lang.NullPointerException: Cannot invoke "org.netbeans.api.visual.widget.LabelWidget.setForeground(java.awt.Color)" because "this.versionW" is null
	at org.netbeans.modules.java.graph.NodeWidget.updatePaintContent(NodeWidget.java:283)
	at org.netbeans.modules.java.graph.DependencyGraphScene.lambda$updateVisibility$1(DependencyGraphScene.java:438)
	at java.base/java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1715)
	at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762)
	at org.netbeans.modules.java.graph.DependencyGraphScene.updateVisibility(DependencyGraphScene.java:438)
	at org.netbeans.modules.java.module.graph.GraphTopComponent.transitiveCheckBoxActionPerformed(GraphTopComponent.java:376)
	at org.netbeans.modules.java.module.graph.GraphTopComponent.access$700(GraphTopComponent.java:76)
	at org.netbeans.modules.java.module.graph.GraphTopComponent$7.actionPerformed(GraphTopComponent.java:334)
	at java.desktop/javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:1972)
....
	at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:747)
	at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:745)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:86)
	at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:744)
	at org.netbeans.core.TimableEventQueue.dispatchEvent(TimableEventQueue.java:136)
[catch] at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
	at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

mbien
mbien approved these changes Mar 17, 2023
View reviewed changes
Copy link
Member

@mbien mbien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Learned something new. I like this graph.

@neilcsmith-net
Copy link
Member Author

Thanks @mbien Hadn't noticed that NPE with readability - wasn't an issue with the project I was testing the graph with, but have replicated on another one. This is a useful graph but could do with some additional work for sure. The JDK module filtering also doesn't seem to work correctly. Will merge this so the graph works at all, then we can work out what else needs fixing! 😄

@neilcsmith-net neilcsmith-net merged commit 755e9d3 into apache:master Mar 17, 2023
@neilcsmith-net neilcsmith-net mentioned this pull request Jul 6, 2023
Merged
@neilcsmith-net neilcsmith-net deleted the gh4227 branch January 16, 2024 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbien mbien mbien approved these changes

@jlahoda jlahoda Awaiting requested review from jlahoda

Assignees
No one assigned
Labels
Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form)
Projects
None yet
Milestone
NB18
Development

Successfully merging this pull request may close these issues.

Module graph fails to display
2 participants
@neilcsmith-net @mbien